Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: correctly calculating free cell #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dabarsch
Copy link

The free cell is calculated somewhere off the map. Since unvisited cells return -1 which is lower than the fullness threshold it wasn't dramatically influencing the scoring. Moreover in icpStep and score the factor map.getDelta() was multiplied twice.

@wjwwood
Copy link
Member

wjwwood commented Jul 29, 2015

@dabarsch thanks for the pr! I don't really have the time, nor the internal understanding of gmapping to review your change. Maybe @vrabaud or @mikeferguson will have time to look at it, but you may have to be patient since I know they're extremely busy as well. We'll get to it as soon as we're able.

@vrabaud
Copy link

vrabaud commented Aug 4, 2015

@mikeferguson , I am trying to quantify the changes: would the entropy be the right measure to look at on some datasets ?

@vrabaud
Copy link

vrabaud commented Aug 8, 2015

ok, I'd like some feedback here: overall, it seems the entropy is lower on the test datasets. Which is good and seems to show your fix is good.

Now, it makes things slower (like 10/20%). @dabarsch , any feedback on that ?

Also, could you please respect the tabs in your commits ? (yes, I knows, it should be spaces but let's respect the code). Also, could you please add comments about what those structures are: I kindof understand your fix but I am not completely sure. Please forgive my ignorance, I am just a maintainer :)

@vrabaud
Copy link

vrabaud commented Aug 8, 2015

this entropy is bs, it's only based on weights: after re-sampling, it equals to the same value for any case .... We need a better measure of quality here.

@topin89
Copy link

topin89 commented Nov 3, 2019

this entropy is bs, it's only based on weights: after re-sampling, it equals to the same value for any case .... We need a better measure of quality here.

Or a flag.

Copy link
Contributor

@k-okada k-okada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dabarsch could you create new PR against melodic-devel branch? we have switched from master to melodic-branch when we changed licence to BSD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants